feat(feed): request wrapping markdown on flag-on GETs - #4823
jmcbgaston wants to merge 2 commits into
Conversation
Flag-off and older clients omit the key so those reads still unwrap. Never send false.
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
greg-in-a-box
left a comment
greg-in-a-box
left a comment
There was a problem hiding this comment.
Summary
Wiring enable_rich_text=true only when activityFeed.richText.enabled is on (and omitting the key when off) looks right for the Feed / ActivitySidebar / FileActivities / ThreadedComments list paths. Never sending false matches the unwrap contract.
Requested changes
-
Annotation thread GET still omits the param.
Annotations.getAnnotationnow acceptsshouldEnableRichText, butuseAnnotationAPI.handleFetchstill calls it without that argument (onlyshouldFetchReplies: true). With the flag on, feed list/reply GETs can return wrapped markdown while opening an annotation thread re-fetches unwrapped content. Please threadactivityFeed.richText.enabledinto that fetch (and any sibling annotation GET that should stay consistent), or call out in the PR why annotation-thread reads intentionally stay on the unwrap path. -
True-path coverage is thin. New tests mostly assert the default
false/ omit path. Please add at least onefeedItems/ ActivitySidebar case that passesshouldEnableRichText: trueand asserts it reaches Annotations / ThreadedComments / FileActivities, plus a unit test forgetAnnotation(..., shouldEnableRichText: true)since that signature changed.
Nits (non-blocking)
- Legacy
fetchComments(non-replies) still has no rich-text param; fine if V2 always goes through threaded/UAA, but worth a one-line note in the description. - Feed cache key is still file-id only; toggling the flag without destroying the Feed instance could serve a stale wrap/unwrap shape (existing pattern, low risk if the flag is stable per mount).
Happy to re-review once the annotation-thread path (or an explicit deferral) is sorted.
2 participants
Description
Activity Feed V2 GETs append
enable_rich_text=truewhenactivityFeed.richText.enabledis on, and omit the key when it is off. Optional last args on the GET helpers so existing callers default to omit. Create and update stay param-free.Quirk: do not send
enable_rich_text=false. Absent key is the unwrap path for flag-off and older clients.